Add zero-checks to axpy-like operations #1573
Conversation
This prevents NaNs from polluting the output
ginkgo-bot
added
reg:testing
| [&beta_val](const type& x) { | ||
| return is_zero(beta_val) ? zero(beta_val) : beta_val * x; | ||
| }); |
There was a problem hiding this comment.
minor performance comment, you could try switching the lambda and zero check, i.e.
is_zero(beta)
? [&beta_val](const type& x) { return zero(beta); }
: [&beta_val](const type& x) { return beta_val * x; }But this might not work, since the two branches of the ?: operator have different types. And it might increase compile times, since it might compile the kernel two times
| arithmetic_type result = c->at(row, j); | ||
| result *= beta_val; | ||
| arithmetic_type result = | ||
| is_zero(beta_val) ? zero(beta_val) : beta_val * c->at(row, j); |
There was a problem hiding this comment.
| is_zero(beta_val) ? zero(beta_val) : beta_val * c->at(row, j); | |
| is_zero(beta_val) ? zero(beta_val) : beta_val * static_cast<arithmetic_type>(c->at(row, j)); |
to make it compile
|
Is there any reason to avoid propagation of NaN? If there's no performance penalty, I think propagation of NaN is easier to know the algorithm does not work out due to some arthmetic error. |
|
@yhmtsai if you are computing for example |
|
no, 0 * NaN should be NaN not zero, so it is not mathimatical equality by just leaving them out. |
|
I know current vendor library usually treat 0 as do not touch due to BLAS. |
that makes calculations more fragile, we already do similar things (special cases) for zeros inside our solver kernels |
|
@yhmtsai we treat |
Operations like$\alpha A x + 0 y$ may propagate NaNs from y to the output despite the 0 coefficient. This can be avoided by checking the beta scaling factors for zero explicitly.
TODO: